Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add monitors #31

Merged
merged 15 commits into from
Nov 29, 2024
Merged

Add monitors #31

merged 15 commits into from
Nov 29, 2024

Conversation

aidnem
Copy link
Contributor

@aidnem aidnem commented Nov 7, 2024

Adds:

  • coppercore.monitors.Monitor
  • coppercore.wpi_interface.MonitoredSubsystem
  • unit tests for monitor

@aidnem aidnem linked an issue Nov 7, 2024 that may be closed by this pull request
Add Monitor and MonitoredSubystem to WPILIb Interface #24
Closed
jkleiber
jkleiber reviewed Nov 13, 2024
View reviewed changes
Copy link
Member

@jkleiber jkleiber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first pass feedback

monitors/src/main/java/coppercore/monitors/Monitor.java Outdated Show resolved Hide resolved
monitors/src/main/java/coppercore/monitors/Monitor.java Show resolved Hide resolved
monitors/src/main/java/coppercore/monitors/Monitor.java Outdated Show resolved Hide resolved
monitors/src/test/java/MonitorTests.java Outdated Show resolved Hide resolved
wpi_interface/build.gradle Outdated Show resolved Hide resolved
wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java Outdated Show resolved Hide resolved
wpi_interface/src/main/java/coppercore/wpi_interface/MonitoredSubsystem.java Outdated Show resolved Hide resolved
aidnem and others added 14 commits November 25, 2024 14:37
@aidnem
WIP - try to port monitors but the packages can't see each other yet …
82aca61 
…so it's all broken
@aidnem
Fix MonitoredSubsystem but its still broken
1c31c7b
@aidnem
Try to add AdvantageKit, now none of my imports work instead of only …
9588742 
…some of them
@aidnem
Turns out excluding wpilibj made us unable to import wpilibj, now jus…
252b4a8 
…t need to fix other import issues
@aidnem
Add builder pattern and fix tests for Monitor
9bc953b
@aidnem
Fix WPI imports and MAYBE fix advantagekit, must test out of codespac…
89ded93 
…e to confirm
@aidnem
GREAT SCOTT GLORY HALLELUJAH IT BUILDS
b6d5b14
@aidnem
Remove to-do comment that had already been done
501019d
@aidnem
Simplify boolean logic in Monitor and improve code comments
aad9f84
@aidnem
Update normal comments to be doc comments for fields of Monitor
9a52604
@aidnem
Remove gradleRIO and manually add wpilib dependencies instead
19ea93d
@aidnem
Add ability to enable and disable logging for MonitoredSubsystem and …
6b310db 
…for each individual Monitor
@aidnem
Make MonitoredSubsystem an abstract class to force users to override …
70be76e 
…monitoredPeriodic
@aidnem
Add tests for monitor fault callbacks
79b80ff
@aidnem aidnem requested a review from jkleiber November 26, 2024 00:04
jkleiber
jkleiber approved these changes Nov 27, 2024
View reviewed changes
Copy link
Member

@jkleiber jkleiber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved, unit tests are passing is my verification evidence

@jkleiber jkleiber merged commit 3125eec into main Nov 29, 2024
6 checks passed
@jkleiber jkleiber deleted the add-monitors branch November 29, 2024 18:45
@aidnem
Copy link
Contributor Author

aidnem commented Nov 30, 2024

Sorry, I didn't see this approval or I would've merged it myself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkleiber jkleiber jkleiber approved these changes

@linglejack06 linglejack06 Awaiting requested review from linglejack06

@avidraccoon avidraccoon Awaiting requested review from avidraccoon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Monitor and MonitoredSubystem to WPILIb Interface
2 participants
@aidnem @jkleiber